Adding dynamic, configurable, meta component, to page template #73

hutchgrant · 2019-05-02T10:00:21Z

Related Issue

Resolves #5

Summary of Changes

Added a components folder to default templates directory - was needed anyway if we wanted to add any other basic components to our default page/app templates.
Added a new meta component which can take the following object example:

{
  title: 'sometitle'
  meta: [
      { property: 'og:site', content: 'greenwood' },
      { name: 'twitter:site', content: '@PrjEvergreen' )
  ]
}

meta component will add title element and array of meta elements
can override title only for now via front-matter on individual pages
all titles include opengraph title meta (og:title)
webpack config path adjustment added a exception for relative paths for default templates(to support adding components to our default template).
greenwood.config.js can include global meta(but it will be overridden if front-matter contains a title for example).
og:url and og:title meta are added by default

greenwood.config.js

module.exports = {
  workspace: 'src',
  publicPath: '/',
  devServer: {
    port: 1984,
    host: 'http://localhost'
  },
  title: 'my pages',
  meta: [
    { property: 'og:site', content: 'greenwood' },
    { name: 'twitter:site', content: '@PrjEvergreen ' }
  ]
};

doubled the time for the tests because my laptop would fail with 15000
every page-template which intends to use meta needs to include METAIMPORT and METADATA at top, as well as METAELEMENT somewhere in the render. Like this:

import { html, LitElement } from 'lit-element';
MDIMPORT;
METAIMPORT;
METADATA;

class PageTemplate extends LitElement {
  render() {
    return html`
      METAELEMENT
      <div class='wrapper'>
        <div class='page-template content'>
          <entry></entry>
        </div>
      </div>
    `;
  }
}

customElements.define('page-template', PageTemplate);

when built, the scaffold.js will replace those variables like so:

        result = result.replace(/METAIMPORT/, `import '${metaComponent}'`);
        result = result.replace(/METADATA/, `const metadata = ${JSON.stringify(metadata)}`);
        result = result.replace(/METAELEMENT/, '<eve-meta .attributes=\${metadata}></eve-meta>');

Problems

* webpack import path adjustment has a problem related to the import of the meta component. I'd prefer users not have to supply one when using custom page templates. Also absolute paths are bugged, had to hardcode a relative one. Will need to work on this issue

* for now need to inclue meta.js within all user workspace component folders

* needs tests

~~To be clear this is a good base to begin with. The meta component maybe should also include the window.location as og:url by default.~~

TODO

Another day, we may want to enable additional front-matter meta overrides within graph.js

thescientist13

Let's try using HtmlWebpackPlugin to template in the <meta> tags by fleshing out all the values in the graph, as it could be an easier way in then a new JS file.

Could also open up possibilities for more ways to inject additional HTML from us or users, like Analytics code.

…ask/fix-issue-5-metadata

hutchgrant · 2019-05-08T21:53:44Z

We cannot simply do HTMLreplacement, because that would work for only a single html file, whereas we would have many html files for each route, each with dynamic meta data. We cannot do this in webpack, I should have realized this earlier when we discussed it. That's why I originally had a component handle setting the meta data as it does in helmet.

The alternative is to have it do it during serialization, by modifying the dom with jsdom. That unfortunately will only run in production, and therefore I think that's not feasible. People need that meta in development, which doesn't run the serialization on dev build.

Back to square one.

hutchgrant · 2019-05-08T22:04:48Z

We've been avoiding having extra components affiliated with the default template, and now we have little choice but to add a meta component. I also prefer the default template to look a little bit better than simply hello world anyway. So this problem, with the NormalModuleReplacementPlugin logic with ./components path vs ../components needs to be addressed, one way or another.

…rehydration, update webpack ModuleReplacement exception

…rehydration, update webpack ModuleReplacement exception, update templates

hutchgrant · 2019-05-09T00:51:06Z

For now, I've hard coded an exception for any imports within cli/templates to be ignored by moduleReplacementPlugin. It's not the cleanest solution, but its pretty simple. It only works with absolute paths, residing in a directory within cli/templates such as ./components/metaComponent, which will be needed in user templates. MetaComponent's path is hard coded to context array and cannot be overridden, which is fine because it's just a utility.

thescientist13

Looks like this PR will need a little TLC post testing refactor PR. But also a good opportunity to add some new cases! 😁

…ask/fix-issue-5-meta

thescientist13

Some interesting things happening here in this PR, seeing some interesting ideas / paths / options for extending Greenwood going forward, taking a lot of notes here 🤓

Left some comments / feedback, but this is looking really good so far!

thescientist13 · 2019-05-14T23:56:13Z

packages/cli/templates/components/meta.js

+*  }
+*/
+
+class meta extends LitElement {


this could be a good candidate as a POC for our "first" (second) package in our (pending) monorepo in terms of coming up with an initial approach for how consumers can extend Greenwood. (markup anyway)

Maybe some sort of configuration where users can provide these placeholder (or Greenwood can provide standard ones, like META) and then we can have plugins for each of these "hooks".

I think it will be good to consider this to help avoid a giant configuration file. Even something like meta could become complex and warrant its own standalone package / API.

My guess is users will include / configure plugins like this and pass it into our configuration.

Just thinking out load, but seems like we could probably get a basic RFC drummed up from this idea for a post Sprint 2 enhancement (basically a "use the platform" version of react-helmet?. 🤔 💡

I thought about this as well, I was tempted to add this to my evergreen web component library That way, there would be no issue with importing it.

thescientist13 · 2019-05-14T23:58:46Z

test/cli/cases/build.config.meta/build.config.meta.spec.js

@@ -0,0 +1,167 @@
+/*


can we break this out into two cases

build.config.title

build.config.meta

Could we also get two additional cases

test empty / default meta values (or maybe update smoke test to test no meta data added?). maybe do an array check?

add error handling case for title

thescientist13 · 2019-05-14T23:59:26Z

.../cli/cases/build.default.workspace-template-app/build.default.workspace-template-app.spec.js

@@ -52,7 +52,7 @@ describe('Build Greenwood With: ', function() {
      it('should have a <title> tag in the <head>', function() {
        const title = dom.window.document.querySelector('head title').textContent;

-        expect(title).to.be.equal('My App');
+        expect(title).to.be.equal('Greenwood App');


I forget, I think we talked about this on the weekly call, by why do we need to change other cases here?

Yea that's the default title within the config.js. So if no title is added to the greenwood.config.js, it will use that one.

…title config error test

thescientist13 · 2019-05-15T01:29:43Z

@hutchgrant
Oops, small issue when updating with master. Looks like the cases from the most recently merged PR needs to have their asserted <title> text updated

  176 passing (2m)
  1 failing

  1) Build Greenwood With: 
       Default Greenwood Configuration and Workspace w/Custom Style Page Template
         Running Smoke Tests: Default Greenwood Configuration and Workspace w/Custom Style Page Template
           Index (Home) page
             should have a <title> tag in the <head>:

      AssertionError: expected 'My App' to equal 'Greenwood App'
      + expected - actual

      -My App
      +Greenwood App
      
      at Context.<anonymous> (test/cli/smoke-test.js:91:29)

hutchgrant · 2019-05-15T01:34:09Z

no its not the text, its the fact the template doesn't have the hooks for the meta component!

thescientist13

Phew! 0.2.0, what a ride! 🎢

hutchgrant added 3 commits April 22, 2019 17:08

fix: adding meta web component

bad3d07

fix: adding meta component to default page template and scaffold

ecfb61e

fix: resolving lint issues

188b18b

thescientist13 self-requested a review May 5, 2019 20:08

thescientist13 reviewed May 5, 2019

View reviewed changes

Merge branch 'master' of github.com:ProjectEvergreen/greenwood into t…

00a056a

…ask/fix-issue-5-metadata

hutchgrant added 3 commits May 8, 2019 20:09

fix: merge config meta with graph, updated meta component to prevent …

883c295

…rehydration, update webpack ModuleReplacement exception

fix: merge config meta with graph, updated meta component to prevent …

3e23368

…rehydration, update webpack ModuleReplacement exception, update templates

fix: remove leftovers from merge

ca68b40

hutchgrant added 6 commits May 8, 2019 21:04

fix: refactor

ba67bad

fix: update meta summary comment

647cc83

test: adding tests for title and meta

fe4a7b4

test: adding more tests for og:url and og:title

2afc21a

fix: small refactor

b772c9a

fix: small refactor

3361a74

thescientist13 added the enhancement Improve something existing (e.g. no docs, new APIs, etc) label May 11, 2019

thescientist13 reviewed May 11, 2019

View reviewed changes

hutchgrant added 4 commits May 11, 2019 19:17

Merge branch 'master' of github.com:ProjectEvergreen/greenwood into t…

3ab9cba

…ask/fix-issue-5-meta

test: adding meta config tests

b10607d

fix: cleanup test description and summary

3de7878

fix: re-enable meta teardown

0599e68

hutchgrant requested a review from thescientist13 May 11, 2019 23:57

thescientist13 reviewed May 15, 2019

View reviewed changes

hutchgrant and others added 3 commits May 14, 2019 21:14

test: seperating meta and title tests, modifying smoke-tests, adding …

b475ba5

…title config error test

test: cleanup

7077836

Merge branch 'master' into task/fix-issue-5-meta

2c08054

thescientist13 assigned hutchgrant May 15, 2019

hutchgrant added 2 commits May 14, 2019 21:36

test: updating custom styled page-template test

416a1d5

test: add meta smoke-test

81cbb33

thescientist13 approved these changes May 15, 2019

View reviewed changes

thescientist13 merged commit cd2fe9a into master May 15, 2019

thescientist13 deleted the task/fix-issue-5-meta branch May 15, 2019 01:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding dynamic, configurable, meta component, to page template #73

Adding dynamic, configurable, meta component, to page template #73

hutchgrant commented May 2, 2019 •

edited

Loading

thescientist13 left a comment

hutchgrant commented May 8, 2019 •

edited

Loading

hutchgrant commented May 8, 2019 •

edited

Loading

hutchgrant commented May 9, 2019 •

edited

Loading

thescientist13 left a comment

thescientist13 left a comment •

edited

Loading

thescientist13 May 14, 2019

hutchgrant May 15, 2019 •

edited

Loading

thescientist13 May 14, 2019

thescientist13 May 15, 2019

thescientist13 May 14, 2019

hutchgrant May 15, 2019

thescientist13 commented May 15, 2019 •

edited

Loading

hutchgrant commented May 15, 2019

thescientist13 left a comment

Adding dynamic, configurable, meta component, to page template #73

Adding dynamic, configurable, meta component, to page template #73

Conversation

hutchgrant commented May 2, 2019 • edited Loading

Related Issue

Summary of Changes

TODO

thescientist13 left a comment

Choose a reason for hiding this comment

hutchgrant commented May 8, 2019 • edited Loading

hutchgrant commented May 8, 2019 • edited Loading

hutchgrant commented May 9, 2019 • edited Loading

thescientist13 left a comment

Choose a reason for hiding this comment

thescientist13 left a comment • edited Loading

Choose a reason for hiding this comment

thescientist13 May 14, 2019

Choose a reason for hiding this comment

hutchgrant May 15, 2019 • edited Loading

Choose a reason for hiding this comment

thescientist13 May 14, 2019

Choose a reason for hiding this comment

thescientist13 May 15, 2019

Choose a reason for hiding this comment

thescientist13 May 14, 2019

Choose a reason for hiding this comment

hutchgrant May 15, 2019

Choose a reason for hiding this comment

thescientist13 commented May 15, 2019 • edited Loading

hutchgrant commented May 15, 2019

thescientist13 left a comment

Choose a reason for hiding this comment

hutchgrant commented May 2, 2019 •

edited

Loading

hutchgrant commented May 8, 2019 •

edited

Loading

hutchgrant commented May 8, 2019 •

edited

Loading

hutchgrant commented May 9, 2019 •

edited

Loading

thescientist13 left a comment •

edited

Loading

hutchgrant May 15, 2019 •

edited

Loading

thescientist13 commented May 15, 2019 •

edited

Loading